Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add codegen ios.componentProvider to package.json #2567

Merged
merged 1 commit into from
Dec 16, 2024
Merged

feat: add codegen ios.componentProvider to package.json #2567

merged 1 commit into from
Dec 16, 2024

Conversation

jakex7
Copy link
Member

@jakex7 jakex7 commented Dec 10, 2024

Description

Add a newly introduced field ios.componentProvider to the codegen configuration to create an association map between JS components and their native implementations.

See more information here: software-mansion/react-native-svg#2572

Changes

  • added codegenConfig.ios.componentProvider field to package.json

Before

Class<RCTComponentViewProtocol> RCTThirdPartyFabricComponentsProvider(const char *name) {
  static std::unordered_map<std::string, Class (*)(void)> sFabricComponentsClassMap = {
    // ...
    {"RNSFullWindowOverlay", RNSFullWindowOverlayCls}, // 3
    {"RNSModalScreen", RNSModalScreenCls}, // 3
    {"RNSScreenContainer", RNSScreenContainerCls}, // 3
    {"RNSScreenContentWrapper", RNSScreenContentWrapperCls}, // 3
    {"RNSScreenFooter", RNSScreenFooterCls}, // 3
    {"RNSScreen", RNSScreenCls}, // 3
    {"RNSScreenNavigationContainer", RNSScreenNavigationContainerCls}, // 3
    {"RNSScreenStackHeaderConfig", RNSScreenStackHeaderConfigCls}, // 3
    {"RNSScreenStackHeaderSubview", RNSScreenStackHeaderSubviewCls}, // 3
    {"RNSScreenStack", RNSScreenStackCls}, // 3
    {"RNSSearchBar", RNSSearchBarCls}, // 3
    // ...

After

@implementation RCTThirdPartyComponentsProvider

+ (NSDictionary<NSString *, Class<RCTComponentViewProtocol>> *)thirdPartyFabricComponents
{
  return @{
		@"RNSFullWindowOverlay": NSClassFromString(@"RNSFullWindowOverlay"), // react-native-screens
		@"RNSModalScreen": NSClassFromString(@"RNSModalScreen"), // react-native-screens
		@"RNSScreenContainer": NSClassFromString(@"RNSScreenContainer"), // react-native-screens
		@"RNSScreenContentWrapper": NSClassFromString(@"RNSScreenContentWrapper"), // react-native-screens
		@"RNSScreenFooter": NSClassFromString(@"RNSScreenFooter"), // react-native-screens
		@"RNSScreen": NSClassFromString(@"RNSScreen"), // react-native-screens
		@"RNSScreenNavigationContainer": NSClassFromString(@"RNSScreenNavigationContainer"), // react-native-screens
		@"RNSScreenStackHeaderConfig": NSClassFromString(@"RNSScreenStackHeaderConfig"), // react-native-screens
		@"RNSScreenStackHeaderSubview": NSClassFromString(@"RNSScreenStackHeaderSubview"), // react-native-screens
		@"RNSScreenStack": NSClassFromString(@"RNSScreenStack"), // react-native-screens
		@"RNSSearchBar": NSClassFromString(@"RNSSearchBar"), // react-native-screens
  };
}

Test code and steps to reproduce

Run pod install in React Native 0.77 app and see RCTThirdPartyFabricComponentsProvider to check the content

@kkafar
Copy link
Member

kkafar commented Dec 10, 2024

Why don't we just use react-native.config.js? Do we need to have all our components declared in both of these places 😄 I remember that the mechanism you described was just recently introduced, maybe we can do something to unify these before 0.77 is released.

I'm thinking right now - it is possible that these fields in react-native.config.js are no longer utilised & we just haven't removed them, but it would be quite nice to get confirmation.

@jakex7
Copy link
Member Author

jakex7 commented Dec 11, 2024

I'm not sure if react-native.config.js is still necessary to generate autolinking.cpp on android, however, the changes mentioned above only impact iOS and have been introduced to remove those functions in the future:

#ifdef RCT_NEW_ARCH_ENABLED
Class<RCTComponentViewProtocol> RNSModalScreenCls(void)
{
  return RNSModalScreen.class;
}
#endif

kkafar
kkafar approved these changes Dec 16, 2024
View reviewed changes
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, this looks solid then. I've glanced at the docs & see that there should be no backward compat issue.

@kkafar kkafar merged commit 7cee79c into main Dec 16, 2024
8 checks passed
@kkafar kkafar deleted the @jakex7/componentProvider branch December 16, 2024 12:34
@kkafar kkafar mentioned this pull request Dec 18, 2024
Merged
8 tasks
kkafar added a commit that referenced this pull request Dec 18, 2024
@kkafar
fix: point to existing symbol names in ios.componentProvider (#2583)
20b7e83 
## Description

#2567 introduced `ios.componentProvider` field to package definition -
and that's fine, but we're pointing to wrong names there.

## Changes

Updated the names to existing symbol names. 


## Test code and steps to reproduce

This caused runtime errors when running the app on 0.77. I need this
commit for the #2581 (0.77 support)

## Checklist

- [ ] Included code example that can be used to test this change
- [ ] Updated TS types
- [ ] Updated documentation: <!-- For adding new props to native-stack
-->
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx
- [ ] Ensured that CI passes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kkafar kkafar kkafar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jakex7 @kkafar